fix: Rework packet loss metric tests to work with UTP 2.0 [MTT-4535]#2174
Merged
simon-lemay-unity merged 4 commits intodevelopfrom Sep 6, 2022
Merged
fix: Rework packet loss metric tests to work with UTP 2.0 [MTT-4535]#2174simon-lemay-unity merged 4 commits intodevelopfrom
simon-lemay-unity merged 4 commits intodevelopfrom
Conversation
johann-r-unity
approved these changes
Aug 31, 2022
jeffreyrainy
approved these changes
Aug 31, 2022
| protected override void OnServerAndClientsCreated() | ||
| { | ||
| var clientTransport = (UnityTransport)m_ClientNetworkManagers[0].NetworkConfig.NetworkTransport; | ||
| clientTransport.DebugSimulatorRandomSeed = 4; // Determined through trial and error. |
Contributor
There was a problem hiding this comment.
Uhhh can you explain this trial and error a little bit
Contributor
There was a problem hiding this comment.
Oops, sorry - I should have read the PR description
Contributor
There was a problem hiding this comment.
I think it might be worth including a comment about why the seed is important, future updates to UTP could change the call pattern to the RNG again and break this test and some poor developer is going to have to retrace your steps
Rosme
approved these changes
Sep 6, 2022
jakobbbb
pushed a commit
to GooseGirlGames/com.unity.netcode.gameobjects
that referenced
this pull request
Feb 22, 2023
…nity-Technologies#2174) * Allow fixing the simulator RNG seed * Fix packet loss test for UTP 2.0 * Add comment about random seed in packet loss test
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The packet loss test is sending 1000 messages and expecting about 25% of them to be dropped. Issue is, these 1000 messages will get batched together into roughly a dozen packets on the wire. By chance, the UTP simulator stage has bug where it always uses the same seed for its RNG. In UTP 1.X, this seed led to 4 packets being dropped, which was enough to get the test to pass. In UTP 2.0, this seed is only getting 2 packets to be dropped (because the call patterns to the RNG are different) and this causes the test to fail.
UTP is getting fixed to not always use the same seed by default, so soon this test will become unstable since it will be at the mercy of the RNG. But UTP does support fixing the seed for just these kinds of scenarios. This PR exposes this functionality to NGO (internal API only) and makes use of it in the packet loss test. The size of the messages being sent is also increased to avoid only sending a dozen packets on the wire.
Changelog
N/A
Testing and Documentation